Change ICP autosave frequency #53
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for querying and configuring the ICP autosave frequency via new genie commands.
- Introduces
get_autosave_freqandset_autosave_freqin the DAE backend, simulation layer, and top-level usercommands - Updates PV name mappings for autosave frequency and its setpoint
- Adds unit tests for the DAE implementation of the new commands
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_genie_dae.py | Adds tests for get_autosave_freq and set_autosave_freq |
| src/genie_python/genie_simulate_impl.py | Stubs autosave_freq member and implements simulation logic |
| src/genie_python/genie_dae.py | Defines real PV-based getters/setters for autosave frequency |
| src/genie_python/genie.py | Registers new usercommands get_autosave_freq and set_autosave_freq |
Comments suppressed due to low confidence (2)
src/genie_python/genie_simulate_impl.py:1095
- Add a docstring to
get_autosave_freqexplaining what the simulation returns and any assumptions (e.g., default units or behavior).
def get_autosave_freq(self) -> "PVValue":
src/genie_python/genie_simulate_impl.py:1095
- There are no unit tests covering the simulation implementation of
get_autosave_freq/set_autosave_freq. Adding tests here would ensure consistency between simulated and real behavior.
def get_autosave_freq(self) -> "PVValue":
|
@Chsudeepta this was closed instead of merged; was that intentional? |
|
@Tom-Willemsen Thanks for pointing, seems I did the same crime twice yesterday. I am reopening the PR |
|
I wasn't keen on |
Changed the return type to int or None
FreddieAkeroyd
force-pushed
the
autosave_freq
branch
from
21cecf7 to
f378504
Compare
Implements two new genie commands, for retrieving autosave frequency and setting it. Relies on ISISComputingGroup/EPICS-isisdae#98